Feat/snowflake ddl sql import solve #789 and #885 schema.description and properties.description on snowflake and postgresql #790
Conversation
|
Can you elaborate on the error with sqlglot? We'd rather want to fix the issue there and not re-introduce the previously removed library. |
|
I found the `AUTOINCREMENT START # INCREMENT [NOORDER|ORDER] do sqlglot fail, a PR is send to them (sqlglot team already) tobymao/sqlglot#5223, |
…d).lint(enabled_linters=set()).has_passed()
|
@simonharrer Can I ask for a review, the comment on snowflake are now well supported for snowflake, and simpleddl is removed. |
…aresma/datacontract-cli into feat/snowflake_ddl_sql_import
|
@simonharrer is it possible to merge this fix for snowflake sql ddl import for comment, tags ? |
|
I include a fix for the T-SQL money data type to decimal related to #751 |
|
@simonharrer please review this PR, it's 2 month old already and solve attributes and table comment |
|
I'll adapt soon the move from DataContractSpecification to ODCS , re-introduce table tags (snowflake) and table comment (description), add timestamp as logicaltype as 3.1 allows it. |
dmaresma
changed the title
…owflake_ddl_sql_import
…dead code
- Fix remove_variable_tokens to correctly substitute all placeholder styles
- Return None instead of string "None" from to_dialect
- Initialize table_description/table_tags to avoid UnboundLocalError
- Remove unused map_physical_type function and format parameter
- Remove duplicate startswith("number") branch in map_type_from_sql
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…extraction The previous code called find(Property) on a Tags node, which returned the Tags node itself (not individual Property children), then relied on iterating the Tags node yielding its expressions. Use tags.expressions directly for correctness. Also fixes get_tags return type annotation (list[str] | None, not str | None) and renames shadowed variable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Hi @dmaresma, I used Claude to refactor |
map_type_from_sql now returns a tuple so callers can pass format through
to create_property(). Fixes several wrong type mappings:
- binary types (BLOB, RAW, BYTEA, VARBINARY…) → string with format "binary"
- UNIQUEIDENTIFIER → string with format "uuid"
- DATETIME/DATETIME2/DATETIMEOFFSET/SMALLDATETIME → timestamp (was date)
- TIME → time (was string)
- endswith("int") no longer catches POINT
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
I used the opportunity to add a |
|
By the way, please consider splitting up different changes into multiple pull requests in the future. This makes it easier to integrate them - long-running PRs with changes at different places take more time to check and are unlikely to be merged timely. |
4 participants
Hi, I use snowflake as main EDW workspace, I notice some regression when simple-ddl-parser was replaced by sqlglot unfortunatly snowflake ddl import is a show stopper for me, in the migration of the catalog into datacontracts.
I take care to maintain sqlglot / sqlserver fonctionnality as is. the sql server test pass.
regression on tags, ddl comment/description, and data type